Skip to content

gh-159493: insert super instructions after borrow analysis in compiler - #160032

Merged
markshannon merged 5 commits into
python:mainfrom
kumaraditya303:compiler-bug
Oct 22, 2026
Merged

gh-159493: insert super instructions after borrow analysis in compiler#160032
markshannon merged 5 commits into
python:mainfrom
kumaraditya303:compiler-bug

Conversation

@kumaraditya303

@kumaraditya303 kumaraditya303 commented Oct 21, 2026 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Contributor

@bedevere-app bedevere-app Bot added the type-feature A feature request or enhancement label Oct 21, 2026
@bedevere-app bedevere-app Bot mentioned this pull request Oct 21, 2026
Closed
@kumaraditya303 kumaraditya303 removed the type-feature A feature request or enhancement label Oct 21, 2026
@eendebakpt

eendebakpt commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Moving the super instructions after the borrow analysis indeed seems like the right thing to do. I gathered some numbers of opcodes generated:

Opcode main PR #160032 borrow branch PR + borrow
STORE_FAST_LOAD_FAST 1,959 66 0 0
STORE_FAST_LOAD_FAST_BORROW 0 0 1,780 1,975
STORE_FAST_STORE_FAST 9,970 11,888 9,970 11,888
LOAD_FAST_LOAD_FAST 994 907 994 907
LOAD_FAST_BORROW_LOAD_FAST_BORROW 42,434 45,615 42,624 45,421
LOAD_FAST (strong) 20,589 20,671 20,655 20,737
LOAD_FAST_BORROW 539,494 535,117 539,227 533,530
STORE_FAST 153,407 151,464 153,586 149,555
all instructions 5,099,985 5,096,866 5,099,974 5,095,151

(borrow branch is main...eendebakpt:cpython:store-fast-load-fast-borrow)

The number of remaining STORE_FAST_LOAD_FAST is very low with this PR. Can you convert it to a STORE_FAST_LOAD_FAST_BORROW? (either this PR or a followup)

eendebakpt
eendebakpt reviewed Oct 21, 2026
View reviewed changes
Comment thread Python/flowgraph.c Outdated
@kumaraditya303

kumaraditya303 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

Can you convert it to a STORE_FAST_LOAD_FAST_BORROW? (either this PR or a followup)

I think to do that in a followup.

@kumaraditya303
kumaraditya303 marked this pull request as ready for review October 21, 2026 10:33
@bedevere-app bedevere-app Bot added the type-feature A feature request or enhancement label Oct 21, 2026
@kumaraditya303 kumaraditya303 removed the type-feature A feature request or enhancement label Oct 21, 2026
eendebakpt
eendebakpt reviewed Oct 21, 2026
View reviewed changes
Comment thread Misc/NEWS.d/next/Core_and_Builtins/2026-10-21-14-00-00.gh-issue-159493.sfLf1b.rst Outdated
kumaraditya303
kumaraditya303 commented Oct 21, 2026
View reviewed changes
Comment thread Misc/NEWS.d/next/Core_and_Builtins/2026-10-21-14-00-00.gh-issue-159493.sfLf1b.rst Outdated
eendebakpt
eendebakpt reviewed Oct 21, 2026
View reviewed changes
Comment thread Python/flowgraph.c Outdated
Comment thread Python/flowgraph.c
Comment thread Python/flowgraph.c
eendebakpt
eendebakpt approved these changes Oct 22, 2026
View reviewed changes

@eendebakpt eendebakpt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good from my side. But I am not very familiar here, so another review would be good to have.

Comment thread Python/flowgraph.c
@markshannon

markshannon commented Oct 22, 2026

Copy link
Copy Markdown
Member

@eendebakpt Where are the instruction frequency numbers from?

@markshannon

markshannon commented Oct 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

I suspect that it isn't worth adding STORE_FAST_LOAD_FAST_BORROW after this PR.

Edit: it isn't worth adding STORE_FAST_LOAD_FAST_BORROW as well as STORE_FAST_LOAD_FAST. We should either replace STORE_FAST_LOAD_FAST or remove it.

markshannon
markshannon approved these changes Oct 22, 2026
View reviewed changes

@markshannon markshannon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@markshannon
markshannon merged commit 4af1265 into python:main Oct 22, 2026
60 checks passed
@kumaraditya303
kumaraditya303 deleted the compiler-bug branch October 23, 2026 05:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eendebakpt eendebakpt eendebakpt approved these changes
@markshannon markshannon markshannon approved these changes
@iritkatriel iritkatriel Awaiting requested review from iritkatriel iritkatriel is a code owner
@Eclips4 Eclips4 Awaiting requested review from Eclips4 Eclips4 is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kumaraditya303 @eendebakpt @markshannon